Enable React concurrent mode #42592

Kicu · 2024-05-24T13:31:22Z

Glossary

React concurrent mode

Starting from react 18 we can run React in new concurrent way (which seems the way React team is moving). Concurrent mode enables a lot of new behaviours in react, most importantly renders can be interrupted by React, re-run or run more than once. This is supposed to make react more performant and webapps more responsive to user actions.
State of this in Expensify:

on native apps after we migrated new arch and enabled Fabric - to my knowledge - we should already be running react concurrently
on web we are running legacy mode and this PR enables concurrent mode

React StrictMode

<StrictMode> is a component that wraps the whole App in (or parts of App) and it runs extra checks and extra behaviors only in dev. So in essence it's a kind of devtool for developers, that should help catch tricky bugs that could be introduced when running concurrent mode. Mainly it renders every component twice (discarding the first render) and runs all effects twice.
This behaviour ☝️ of StrictMode is the source of most bugs that happen to us.

Note: StrictMode is not needed for concurrent react to work but it is much safer to use it.

https://react.dev/reference/react/StrictMode

Details

This PR enables running React 18+ in concurrent mode alongside using StrictMode.
I did not provide screenshots or videos because this affects the whole how React behaves, so in theory affects the whole app.

Done

react concurrent mode turned on and whole App wrapped in <StrictMode>
react-navigation fixed via patch; see details here: Enable React concurrent mode #42592 (comment)
react-native-reanimated fixed via patch, however it should be released soon as the team is aware of StrictMode problems.
The related upstream PRs are: Fix web exiting animation in StrictMode software-mansion/react-native-reanimated#6129 and Merge ViewDescriptors implementation software-mansion/react-native-reanimated#6123 and reanimated team expects correct version to be released in 2.13

List of known bugs & fixes

~~react-navigation breaks on StrictMode - fixed via local patch - need to verify the legitness of said patch~~ solved
react-native-reanimated
- animations not working - OS team will fix, we have local patch in the meantime
- ~~initial animation when entering Report sometimes flickers; this happens because of StrictMode so only in dev~~

Fixed Issues

$ #33531
PROPOSAL:

Tests

Verify that web and native build and run
test all the most important features of the app and verify nothing breaks

Offline tests

QA Steps

verify that web and native app runs
test all the most important features of the app and verify that nothing obvious breaks

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

melvin-bot · 2024-05-24T13:31:26Z

Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers!

Kicu · 2024-05-24T13:33:55Z

src/components/TextInput/TextInputClearButton/index.tsx

@@ -40,4 +40,4 @@ function TextInputClearButton({onPressButton}: TextInputClearButtonProps) {

 TextInputClearButton.displayName = 'TextInputClearButton';

-export default forwardRef(TextInputClearButton);


This is unrelated but a source of warning in browser console - the ref was unused.
While working on updated React I wanted to get rid of as many console warnings as I can since it helps to find the actual React bugs

Sounds good, but may I suggest creating a separate PR to remove this ref?

Can you please revert this change as it's done in #44155

Kicu · 2024-06-19T14:16:35Z

`react-navigation` status

react-navigation in version 6 (current stable) does not work well in StrictMode when using linkingConfig - which we do
I created a simple RN app with reproduction of this behaviour and I have tested it using newer v7 version and it works, However react-navigation 7 is still in alpha: https://github.com/react-navigation/react-navigation/releases
I asked @satya164 who is an author of react navigation and he said that it is unlikely for this fix to be backported to 6 and suggested we try version 7 - perhaps this is a good idea to try
I have found the source of bug in v6 and compared the code to v7 and I was able to prepare a patch for expensify. I can't 100% guarantee at this point that the patch won't break something else, but it feels mostly safe - I only edited one file.

detailed description of patch

The source of problems for us is that inside useNavigationBuilder there is code that cleans state on component unmount here: https://github.com/react-navigation/react-navigation/blob/6.x/packages/core/src/useNavigationBuilder.tsx#L557-L561
Because of StrictMode navigation components will be rendered twice and their effects will run twice. This in turn means that when the navigation renders and runs for the second time, the state is already cleaned from the previous effect cleanup.

Then this check will trigger: https://github.com/react-navigation/react-navigation/blob/6.x/packages/core/src/useNavigationBuilder.tsx#L372 and there is no navigation happening at all.

The patch fixes this in a similar way to v7 branch.

CC @adamgrzybowski @WoLewicki for visibility

adamgrzybowski · 2024-06-20T10:03:27Z

As a follow up to @Kicu mentioning react-navigation 7. This version introduces two functionalities that we may be interested in:

preload -> We could potentially use it for optimization and performance.
popTo -> It may help us with simplifying the UP action implementation.

…imated

melvin-bot · 2024-06-20T11:00:51Z

@rayane-djouah Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

Kicu · 2024-06-20T11:04:17Z

@mountiny @roryabraham We're ready for liftoff 🚀

Web and native builds work, there are no crucial issues and app works. Now I think we need some heavy testing by QA's - I tested only the basic functionalities that I know about. If any bugs come up they could be rather tricky or non-obvious.

WoLewicki · 2024-06-20T11:15:20Z

Maybe we could open another PR with all the changes not strictly related to the StrictMode so they can be also tested and merged earlier if they work properly? This way, this PR would focus only on the StrictMode. And that other PR is merged before this one, you can just merge the newest main here. Wdyt @Kicu ?

Kicu · 2024-06-20T11:23:53Z

I'm not sure. Technically it will be easy to split this into only concurrent mode and only StrictMode and I could do it, but how much would this really improve the process?
In theory if QA people will test this on production build, then strictmode should not affect it in any way. Per docs:

All of these checks are development-only and do not impact the production build.

So maybe leaving this PR as is will just be simpler?

jasperhuangg · 2024-07-01T18:36:55Z

@Kicu @roryabraham I'm having trouble following how this code only runs on dev considering that the USE_REACT_STRICT-MODE flag is always true and isn't dependent on any environment variables. Can you explain?

I'm mainly asking because there's a crash happening here on staging that involves a component exceeding the max update depth.

jasperhuangg · 2024-07-01T19:25:06Z

Hmm taking a deeper look at the docs it seems there's logic within React's StrictMode implementation that prevents it from running effects twice on staging/production builds.

Kicu · 2024-07-02T06:39:43Z

@jasperhuangg indeed its the internal behavior of StrictMode, described in the React docs that makes it only work in dev.
https://react.dev/reference/react/StrictMode#enabling-strict-mode-for-entire-app
So I'm trusting react devs here. It should be safe to ship apps wrapped with strict mode to prod and expect strict mode will not do any of the checks.

The CONFIG flag is for the convenience of developers for quick testing locally why their code fails

…current-react-lets-go" This reverts commit d047467, reversing changes made to 04f6598.

OSBotify · 2024-07-03T19:37:12Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.0.3-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2024-07-10T10:54:47Z

🚀 Deployed to production by https://github.com/Julesssss in version: 9.0.5-13 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

ikevin127 · 2024-07-31T23:50:55Z

👋 The introduction of StrictMode with this PR created the following dev issue:

[Pay 7/30] [Dev] - [StrictMode] Attachments not loading and app crashes when click on loading preview from attachment #44566

The issue was fixed by PR #45559, more details on the proposal here.

ishpaul777 · 2024-08-03T16:21:54Z

👋 The introduction of concurrent mode caused Left hand menu and RHP to not open instantly when opening from link

The issue was fixed by PR #45778

ahmedGaber93 · 2024-08-20T16:10:17Z

👋 The introduction of StrictMode cause this DEV issue #44235 and it fixed here #46483

Strict Mode will unmount then remount component on the first render

React 18 introduces a new development-only check to Strict Mode. This new check will automatically unmount and remount every component

see here

Bad
unmount will set value to true, then remount will not back it to false

const isUnmounted = useRef(false);
useEffect(() => {
  return () => {
      isUnmounted.current = true;
  };
}, []);

Good
unmount will set value to true, then remount will back it to false

const isUnmounted = useRef(null);
useEffect(() => {
  isUnmounted.current = false; // remount will back value to false
  return () => {
      isUnmounted.current = true;
  };
}, []);

ntdiary · 2024-10-08T14:10:22Z

src/setup/platformSetup/index.website.ts

@@ -53,7 +53,6 @@ const webUpdater = (): PlatformSpecificUpdater => ({
 export default function () {
    AppRegistry.runApplication(Config.APP_NAME, {
        rootTag: document.getElementById('root'),
-        mode: 'legacy',


Coming from #47041, our app has some infinite Lottie animations. In concurrent mode, when the SidebarScreen is suspended (i.e., Suspense feature), it conflicts with these animations, preventing the animation component from rendering. :)

Kicu mentioned this pull request May 24, 2024

[HOLD for payment 2024-07-22] LOW: Enable Concurrent React Features #33531

Closed

Kicu commented May 24, 2024

View reviewed changes

Kicu force-pushed the kicu/concurrent-react-lets-go branch from 8dd4c17 to b3ff01b Compare May 29, 2024 07:03

Kicu force-pushed the kicu/concurrent-react-lets-go branch 5 times, most recently from 190c401 to 831a491 Compare June 19, 2024 10:29

Kicu changed the title ~~[WIP] Enable React concurrent mode~~ Enable React concurrent mode Jun 19, 2024

Kicu force-pushed the kicu/concurrent-react-lets-go branch from 831a491 to 875756f Compare June 20, 2024 08:05

Kicu added 6 commits June 20, 2024 12:18

Enable React concurrent mode

04c47c9

Add fixes for ReportScreen on useOnyx and patch for react-native-rean…

28952fa

…imated

Add better patch fixing react-navigation in StrictMode

09d0cf1

Cleanup and try fixing reanimated

8aa0174

Remove not needed reanimated patch for strict mode

a29219b

Cleanup patches for react navigation

85250a6

Kicu force-pushed the kicu/concurrent-react-lets-go branch from 875756f to 24fa270 Compare June 20, 2024 10:19

Final cleanups for concurrent mode

a2c8d2c

Kicu force-pushed the kicu/concurrent-react-lets-go branch from 24fa270 to a2c8d2c Compare June 20, 2024 10:59

Kicu marked this pull request as ready for review June 20, 2024 11:00

Kicu requested a review from a team as a code owner June 20, 2024 11:00

melvin-bot bot removed the request for review from a team June 20, 2024 11:00

melvin-bot bot requested a review from rayane-djouah June 20, 2024 11:00

Ollyws mentioned this pull request Jun 28, 2024

[HOLD for payment 2024-07-22] [$250] Web - Chat - Notification preferences RHP does not animate smoothly #44421

Closed

6 tasks

hungvu193 mentioned this pull request Jul 2, 2024

[HOLD for payment 2024-08-07] [$250] mweb - Wrong status bar color at the top in safari #44685

Closed

6 tasks

bondydaa added a commit that referenced this pull request Jul 3, 2024

Revert "Merge pull request #42592 from software-mansion-labs/kicu/con…

a3dd23a

…current-react-lets-go" This reverts commit d047467, reversing changes made to 04f6598.

mountiny added a commit that referenced this pull request Jul 3, 2024

Revert "Merge pull request #42592 from software-mansion-labs/kicu/con…

135c6be

…current-react-lets-go" This reverts commit d047467, reversing changes made to 04f6598.

mountiny mentioned this pull request Jul 3, 2024

Reverts "Enable React concurrent mode" #44779

Closed

50 tasks

ShridharGoel mentioned this pull request Jul 3, 2024

[CHECKLIST - ISHPAUL] [$250] Mac Safari - Tasks/Chat - Second right hand menu pane RHP shown severely cut compared to prod #44765

Closed

6 tasks

Ollyws mentioned this pull request Jul 4, 2024

[HOLD for payment 2024-07-25] [$250] Incorrect animation when accessing Referral information page #44745

Closed

6 tasks

suneox mentioned this pull request Jul 6, 2024

[HOLD for payment 2024-07-24] LHN not displayed after page refresh from settings page #44860

Closed

6 tasks

ishpaul777 mentioned this pull request Jul 11, 2024

[HOLD for payment 2024-08-01] [$250] Settings - Left hand menu and RHP do not open instantly when opening from link #44585

Closed

6 tasks

Kicu deleted the kicu/concurrent-react-lets-go branch July 24, 2024 14:52

ikevin127 mentioned this pull request Jul 31, 2024

[Pay 7/30] [Dev] - [StrictMode] Attachments not loading and app crashes when click on loading preview from attachment #44566

Closed

6 tasks

ahmedGaber93 mentioned this pull request Aug 20, 2024

[HOLD for payment 2024-08-19] [$250] Concierge - Error page when new account navigate to Concierge via Link #44235

Closed

6 tasks

tsa321 mentioned this pull request Aug 22, 2024

Make onboarding continue from last visited onboarding page #47472

Merged

50 tasks

kirillzyusko mentioned this pull request Aug 23, 2024

[$500] Login – No Abracadabra page when logging in via Magic link #44600

Open

6 tasks

Nodebrute mentioned this pull request Aug 26, 2024

[HOLD for payment - waiting on deploy][$250] Opening Two-Factor Authentication settings causes two duplicate API requests for EnableTwoFactorAuth #48034

Closed

6 tasks

Kicu mentioned this pull request Sep 5, 2024

Disable legacy mode in desktop #48645

Merged

48 tasks

kirillzyusko mentioned this pull request Sep 19, 2024

fix: abracadabra page #48899

Open

48 tasks

suneox mentioned this pull request Sep 25, 2024

[PAID] [$250] The preference setting doesn’t work when navigating between chat and settings. #47273

Closed

6 tasks

ntdiary reviewed Oct 8, 2024

View reviewed changes

ntdiary mentioned this pull request Oct 8, 2024

[$250] Web- User avatar & animation do not load after switching from Inbox to Settings with animation #47041

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable React concurrent mode #42592

Enable React concurrent mode #42592

Kicu commented May 24, 2024 •

edited

Loading

melvin-bot bot commented May 24, 2024

Kicu May 24, 2024 •

edited

Loading

roryabraham Jun 21, 2024

rayane-djouah Jun 24, 2024

Kicu commented Jun 19, 2024

adamgrzybowski commented Jun 20, 2024

melvin-bot bot commented Jun 20, 2024

Kicu commented Jun 20, 2024

WoLewicki commented Jun 20, 2024

Kicu commented Jun 20, 2024

jasperhuangg commented Jul 1, 2024 •

edited

Loading

jasperhuangg commented Jul 1, 2024

Kicu commented Jul 2, 2024

OSBotify commented Jul 3, 2024

OSBotify commented Jul 10, 2024

ikevin127 commented Jul 31, 2024

ishpaul777 commented Aug 3, 2024

ahmedGaber93 commented Aug 20, 2024

ntdiary Oct 8, 2024

		@@ -40,4 +40,4 @@ function TextInputClearButton({onPressButton}: TextInputClearButtonProps) {

		TextInputClearButton.displayName = 'TextInputClearButton';

		export default forwardRef(TextInputClearButton);

Enable React concurrent mode #42592

Enable React concurrent mode #42592

Conversation

Kicu commented May 24, 2024 • edited Loading

Glossary

React concurrent mode

React StrictMode

Details

Done

List of known bugs & fixes

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented May 24, 2024

Kicu May 24, 2024 • edited Loading

Choose a reason for hiding this comment

roryabraham Jun 21, 2024

Choose a reason for hiding this comment

rayane-djouah Jun 24, 2024

Choose a reason for hiding this comment

Kicu commented Jun 19, 2024

react-navigation status

detailed description of patch

adamgrzybowski commented Jun 20, 2024

melvin-bot bot commented Jun 20, 2024

Kicu commented Jun 20, 2024

WoLewicki commented Jun 20, 2024

Kicu commented Jun 20, 2024

jasperhuangg commented Jul 1, 2024 • edited Loading

jasperhuangg commented Jul 1, 2024

Kicu commented Jul 2, 2024

OSBotify commented Jul 3, 2024

OSBotify commented Jul 10, 2024

ikevin127 commented Jul 31, 2024

ishpaul777 commented Aug 3, 2024

ahmedGaber93 commented Aug 20, 2024

Strict Mode will unmount then remount component on the first render

ntdiary Oct 8, 2024

Choose a reason for hiding this comment

Kicu commented May 24, 2024 •

edited

Loading

Kicu May 24, 2024 •

edited

Loading

`react-navigation` status

jasperhuangg commented Jul 1, 2024 •

edited

Loading